Skip to content

Conversation

seijikun
Copy link
Contributor

@seijikun seijikun commented Sep 3, 2025

I still had this lying around as experiment.
Polished it up a bit for the merge request.

It's best to review this commit-by-commit.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@seijikun seijikun changed the title Mr hii configrouting uefi: Add bindings for the HII_CONFIG_ROUTING protocol Sep 3, 2025
@seijikun seijikun force-pushed the mr-hii-configrouting branch 4 times, most recently from 6380ed3 to 2e556f1 Compare September 4, 2025 15:05
@seijikun seijikun force-pushed the mr-hii-configrouting branch from 2e556f1 to 0c54e18 Compare September 10, 2025 15:21
Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed everything and it LGTM! Thanks for working on this :) Left a few nits. Please answer my comments and then we are good to go

let size_bytes = data.len() / 2 + 2; // includes \0 terminator
let size_chars = size_bytes / 2;
let mut bfr = AlignedBuffer::from_size_align(size_bytes, 2).ok()?;
bfr.copy_from_iter(Self::parse_bytes_from_hex(data).chain([0, 0]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the [0, 0] some form of termiation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the [0, 0] bytes is a uint16 0.
I added the null terminator so that I could use the CStr16::from_u16_* APIs.
I must admit, I'm somewhat at odds with the entire UTF-16 string API surface of uefi.

};
let value = match splitter.next() {
Some(("VALUE", Some(data))) => Self::parse_bytes_from_hex(data).collect(),
Some(("VALUE", Some(data))) => Self::parse_bytes_from_hex(data).rev().collect(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why is it necessary to rev the bytes here??

Copy link
Contributor Author

@seijikun seijikun Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment:

            // The uefi specification declares `VALUE` as being a "number" of arbitrary length.
            // And numbers have to be endianess-corrected. Thus, the bytes represented by a value's
            // hex string of arbitrary length have to be reversed to account for that weird encoding.

The short form is: "absolutely no idea. I checked it, and it's true" 😆
I've seen it stated somewhere else and was surprised that they were right.
https://github.com/Kostr/UEFI-Lessons/tree/master/Lessons_uncategorized/Lesson_Configuration_Language_5
^ This repo is worth more than gold!

@seijikun seijikun force-pushed the mr-hii-configrouting branch from 0c54e18 to 2977208 Compare September 25, 2025 14:01
@seijikun seijikun force-pushed the mr-hii-configrouting branch from 2977208 to ef2b00d Compare September 25, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants